Skip to content

migrate: rewrite workflows' runs-on labels in place - #156

Merged
linyiru merged 3 commits into
mainfrom
feat/migrate-engine
Nov 4, 2026
Merged

migrate: rewrite workflows' runs-on labels in place#156
linyiru merged 3 commits into
mainfrom
feat/migrate-engine

Conversation

@linyiru

@linyiru linyiru commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Part 1 of #146: the rewrite engine for firerunner migrate. The CLI, which reads a repository's workflows and opens the PR, follows in a second PR.

migrate.Rewrite(src, map) replaces each job's runs-on labels in place, at the position the YAML parser reports, keeping each label's quoting, the comments and every other byte. The YAML is never re-serialized, so the PR shows only the label changes.

Jobs it leaves alone, each reported with a reason:

  • a ${{ }} expression (e.g. a matrix OS)
  • a runner group (group:/labels:)
  • self-hosted
  • any unmapped label in the list (a job's labels are mapped all or none)
  • a reusable workflow call (uses:)
  • anchors, aliases and block scalars

A label that is already a map target counts as migrated, so a second run changes nothing. Before each edit, the text found in the file must equal the parsed value, and the rewritten file must still parse.

Tests:

  • one golden workflow that covers every runs-on shape
  • edge cases: CRLF line endings, no trailing newline, non-ASCII text before a label (yaml.v3 counts columns in characters), escaped quotes, # inside a label
  • validation of the label map
  • mutation-checked: the guards for self-hosted, expressions, unmapped labels, placement, anchors and aliases are each CAUGHT

Adds go.yaml.in/yaml/v3, the maintained successor of the archived gopkg.in/yaml.v3.

@linyiru
migrate: rewrite workflows' runs-on labels in place
8fa941f 
The engine behind `firerunner migrate` (#146): given a label map
(ubuntu-latest=fr-small), it replaces each job's runs-on labels where
they stand in the file, keeping their quoting, comments and every other
byte, rather than round-tripping the YAML, so the PR it opens is a diff
of labels only.

A job it cannot map safely is left alone and reported with why: an
expression, a runner group, self-hosted runners, an unmapped label (all
of a job's labels map or none does), a reusable workflow call, anchors,
aliases and block scalars. A label already on a map target counts as
migrated, so a second run is a no-op. Each edit is checked against the
parsed value before it is made, and the result must parse.

Uses go.yaml.in/yaml/v3, the maintained successor of gopkg.in/yaml.v3.

Signed-off-by: Lawrence Lin <deduce@gmail.com>
@linyiru
migrate: count lines as YAML does, and prove each rewrite
fce2d1e 
Review of #156 found that a lone CR, NEL, LS or PS before a job (in a
comment or a quoted string) shifted yaml's line numbers off ours, so an
edit could land on the next line's identical label: wrong, parseable,
and reported as a change. Lines now split on every YAML break, and a
rewrite is accepted only if the output parses to the same tree with
every scalar unchanged but the planned ones, which hold their new
labels. That check, not span, is what guarantees nothing else moved.

Also from the review:
- refuse a map that chains (a=b, b=c) or names a label twice in two
  cases, and targets YAML reads as a bool, null or number;
- match labels without regard to case, as GitHub does;
- refuse files with several documents or a duplicate job, and skip
  jobs with a merge key, an anchor or alias, or runs-on twice, so none
  is edited or dropped from the report silently;
- clearer reasons for tagged labels, labels-only objects, empty
  runs-on; a leading BOM no longer shifts line 1's columns.

Signed-off-by: Lawrence Lin <deduce@gmail.com>
@linyiru

linyiru commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Independent review (a fresh reviewer agent, given only the diff). Every finding is answered in fce2d1e:

  1. HIGH: an edit could land on the wrong line. yaml counts a lone CR, NEL, LS and PS as line breaks, and the engine counted only \n. One of these characters before a job (in a comment or a quoted string) shifted the line numbers, so the next line's identical label was rewritten: the output still parsed, and the edit was reported as a change. Fixed:
    • Lines are now split on every YAML line break.
    • A rewrite is accepted only if the output parses to the same tree, with every scalar unchanged except the planned ones, which must hold their new labels. This check is the guarantee, independent of how span locates a label.
    • Regression tests use the reviewer's inputs, and verify is tested on its own with misplaced, wrong-text and stray edits.
  2. MEDIUM: a chained map (a=b, b=c) broke idempotence. Fixed: such a map is refused. A map that names one label twice in different cases is refused too.
  3. Unclear skip reasons. Fixed for tagged labels (!!str), labels-only objects and an empty runs-on.
  4. Shapes that were silently mishandled. Fixed:
    • A file with several YAML documents, or with a duplicate job id, is refused as a whole.
    • A job with a merge key, a job anchor or alias, or a duplicate runs-on key is skipped with a reason.
    • A leading BOM no longer shifts the columns on line 1.
  • Targets that YAML reads as typed scalars (true, null, 123, 1.5, 0x10) are refused. (on is a string in YAML 1.2, the version yaml.v3 parses, so it is allowed.)
  • Case: labels now match case-insensitively, as GitHub matches them. This also covers Self-Hosted.

Mutation check: each new guard is CAUGHT (line breaks, BOM, verify's value and stray-change checks, chain, typed target, merge key, duplicate runs-on and duplicate job, multiple documents, tags, case-insensitive self-hosted).

@linyiru
migrate: skip a non-string label's job, and fold ASCII case only
7dc3b85 
From the second review of #156:
- a source label YAML reads as a number or bool (runs-on: 123) failed
  the whole file in verify; it now skips its own job, with why;
- runs-on repeated after another duplicate key went unnoticed;
- strings.EqualFold equates the long s with s, so a map could resolve
  a label two ways; labels now fold ASCII case only, as GitHub does;
- an empty quoted runs-on and an aliased jobs mapping are reported.

Signed-off-by: Lawrence Lin <deduce@gmail.com>
@linyiru

linyiru commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Second review of fce2d1e by the same independent reviewer: lineSpans now matches yaml.v3 on every probe, verify could not be fooled, and the validation rules reject no legitimate label. Its four leftover findings are fixed in 7dc3b85, each with a test and a CAUGHT mutant:

  • a numeric or boolean source label now skips only its own job (it used to fail the whole file);
  • a duplicate runs-on that comes after another duplicate key is now detected;
  • labels fold ASCII case only, so the long s (ſ) no longer matches s;
  • an empty quoted runs-on and an aliased jobs mapping are now reported.

Not changed: on/yes/no stay accepted as targets. YAML 1.2, which yaml.v3 parses, reads them as strings, and so does GitHub (its own on: key).

@linyiru
linyiru merged commit 9bf9f16 into main Nov 4, 2026
7 checks passed
@linyiru
linyiru deleted the feat/migrate-engine branch November 4, 2026 22:32
This was referenced Nov 4, 2026
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@linyiru